fix: make hevy-mcp CLI start stdio server again - #184
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 2 2
=========================================
Hits 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall, the refactor cleanly separates the library server entry (index) from the CLI entry (cli) and removes the brittle isDirectExecution heuristic without introducing obvious bugs. The main concern is a minor maintainability issue: src/cli.ts imports "./index.js" directly, which couples the TS source to the emitted .js extension and could cause fragility if the build configuration changes. There is also an implicit coupling between package.json's bin.hevy-mcp path and tsup.config.ts that may be worth documenting to avoid future drift. No correctness or performance regressions are evident in the diff.
Additional notes (2)
-
Maintainability |
package.json:16-20
The change to point thebinentry atdist/cli.jsand include the tsupbannerwith the shebang looks consistent, but it tightly couples the published binary path (dist/cli.js) to the build config. If the build output filename or directory changes in the future, thebinmapping will silently drift. While not a bug now, it’s worth considering consolidating this mapping (for example via a single source of truth or a small script) so the CLI path can’t get out of sync with tsup’s output. -
Maintainability |
src/index.ts:54-54
ExportingrunServerfrom the library entry makes sense for reuse, but it also exposes a function whose primary concern is process‑level side effects (readingprocess.argv,process.env, and exiting via errors). That blurs the boundary between the pure “library” surface (createServer,configSchema) and CLI concerns, and may surprise downstream consumers who expectindexexports to be side‑effect‑free utilities.
A cleaner separation would keep index focused on pure or configuration‑driven helpers and move the process/CLI wiring (argument parsing, env inspection, logging) into cli.ts, possibly by extracting a lower‑level helper that cli.ts calls. That would make it clearer which exports are safe for embedding vs. which are intended for top‑level CLI usage.
Summary of changes
Summary of Changes
- Updated
package.jsonto point thehevy-mcpbinary and related scripts (inspect,start,dev) to a newdist/cli.jsentrypoint instead ofdist/index.js. - Added a new
src/cli.tsfile that importsrunServerfrom the library entry and executes it with top-level error handling. - Refactored
src/index.tsto:- Export
runServeras a named function. - Remove the
isDirectExecutionheuristic and inline bootstrapping logic.
- Export
- Extended
tsup.config.tsto build bothsrc/index.tsandsrc/cli.tsintodist/with the existing Node shebang banner.
|
@CharlieHelps can you add tests for your work and create a github action which uses the hevy api token secret we have set in github |
Expand this to see my work.
|
|
I’ve added tests around the new stdio entry behavior and confirmed the existing GitHub Actions already use the Hevy API key secret for integration runs. Changes
If you’d prefer a dedicated, CLI-focused workflow on top of the existing ones (e.g., a small smoke test that starts Verification# Build: OK
pnpm run build
# Biome check (auto-fix enabled): OK
pnpm run check
# Vitest: unit + entrypoint tests pass; integration suite fails locally without HEVY_API_KEY (expected)
pnpm test
|
There was a problem hiding this comment.
The diff cleanly separates the CLI entry from the library entry and adds solid tests for runServer, but there are a couple of maintainability concerns. src/cli.ts hard-codes an "./index.js" import path, which couples the TypeScript source to the emitted JS extension and could break if the build config changes. The runServer export from index.ts blurs the line between library and CLI concerns by embedding process-level side effects into the library surface. Test helpers around StdioServerTransport rely on a special __transports export from the mocked module; while functional, this is a bit fragile and could be simplified by keeping the tracking array local to the test file.
Additional notes (3)
- Maintainability |
src/index.test.ts:25-46
The newrunServertests mutateprocess.envandprocess.argvand then restore them inbeforeEach/afterEach, which is good, but they also rely on the importedstdioModulemock having an__transportsarray that’s cleared by slicing itslengthback to 0. This pattern couples the tests to an internal testing-only export (__transports) on the mocked module, which is a bit fragile if the mock implementation ever changes or if someone reuses@modelcontextprotocol/sdk/server/stdio.jsmocks elsewhere.
To keep the tests simpler and less coupled to the transport mock’s shape, you could assert directly on construction or connect calls via spies on McpServer.prototype.connect, or export the transports test helper directly from the test file instead of attaching it to the module namespace. As-is, it works, but the indirection through stdioModule is easy to break inadvertently during refactors.
- Maintainability |
src/index.test.ts:40-53
The test suite mutatesprocess.envandprocess.argvin multiple places and attempts to restore them inbeforeEach/afterEach. This is generally fine, but it is brittle if additionaldescribeblocks or tests are added that also modify these globals without going through the same helpers.
Centralizing env/argv manipulation in small helpers (e.g., setEnv, setArgv) or a single shared test utility would reduce the risk of future tests forgetting to restore state or conflicting with these expectations, and make it clearer what global state each test depends on.
- Maintainability |
src/index.ts:54-54
runServeris now exported from the main library entry (index.ts) while also being used as the CLI bootstrap. That means consumers importing from the package’s main entry get a function that readsprocess.argv/process.env, writes tostderr, and exits the process on error. This mixes library concerns with process-level side effects and may be surprising to downstream users who treatindexas a pure library surface.
The new cli.ts file is already a dedicated entrypoint; pushing more of the process/CLI wiring (argument parsing, logging, process.exit) into cli.ts and keeping index.ts focused on configuration-driven server construction (buildServer, createServer) would make the boundary clearer and reduce the risk of runServer being misused in embedded contexts.
Summary of changes
Summary of Changes
- Switched the npm
binmapping and relevant scripts inpackage.jsonfromdist/index.jsto a newdist/cli.jsentry, and updateddevto watchsrc/cli.ts. - Added a new
src/cli.tsfile that importsrunServerfrom the main module and invokes it with top-level error handling. - Refactored
src/index.tsto exportrunServerexplicitly and removed theisDirectExecutionheuristic that auto-started the server on direct execution. - Expanded
tsup.config.tsto build bothsrc/index.tsandsrc/cli.tswith the existing Node shebang banner. - Significantly extended
src/index.test.tsto mock the MCP stdio transport and Hevy client, and to add focused tests aroundrunServer’s behavior (env vs CLI config, missing API key, stdio transport connection).
| @@ -0,0 +1,6 @@ | |||
| import { runServer } from "./index.js"; | |||
There was a problem hiding this comment.
src/cli.ts is TypeScript source but imports "./index.js", which bakes the emitted .js extension into the source path. That’s brittle against future changes in build configuration (e.g., different extension, multiple output formats, or non-relative resolution) and makes this file behave differently under tsx/ts-node vs compiled output. Since the rest of the codebase uses extension-less internal imports in TS (e.g., "./utils/hevyClient.js" in tests is fine because it only runs under Node+ts-node/tsx test environment), this one stands out as a maintainability risk for the CLI entrypoint specifically.
Using the module name without the extension ("./index") keeps the TS source decoupled from a specific JS extension while still resolving correctly through tsup and Node’s ESM loader in the built output.
Suggestion
Consider changing the import in src/cli.ts to avoid hard-coding the .js extension, so the TS source does not depend on the emitted filename details:
import { runServer } from "./index";This keeps the CLI entry resilient if you later tweak tsup output options (e.g., different extension or dual CJS/ESM builds). Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
## [1.12.13](v1.12.12...v1.12.13) (2025-12-05) ### Bug Fixes * make hevy-mcp CLI start stdio server again ([#184](#184)) ([3fd898a](3fd898a))
2 participants
Ensure the
hevy-mcpstdio server actually starts when invoked via the npm CLI (npx hevy-mcp) by introducing a dedicated CLI entrypoint and standardizing scripts around it.Changes
src/index.tsfocused on exportingcreateServer,configSchema, and a reusablerunServerhelper.src/cli.tswrapper that simply imports and runsrunServer.bin.hevy-mcptodist/cli.jsinstead ofdist/index.js.startandinspectscripts to runnode dist/cli.jsso local flows use the same code path asnpx hevy-mcp.devto watchsrc/cli.ts, keeping the dev server behavior consistent with the CLI.tsupconfig to build bothsrc/index.ts(library) andsrc/cli.ts(CLI) intodist/.This removes the brittle
isDirectExecutionheuristic on the bundledindexentry and makes the CLI startup behavior explicit and reliable for tools like Cursor, Claude Desktop, and Smithery that shell out tonpx hevy-mcp.Verification
HEVY_API_KEY=dummy node dist/cli.jsto confirm it initializes the Hevy client and starts the stdio server without crashing.HEVY_API_KEY.Closes #181
✨ PR Description
Purpose: Fix the hevy-mcp CLI by creating a dedicated entry point that properly starts the stdio server.
Main changes:
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how